pkg/apiserver/rest/dualwriter_mode3_test.go (3)

119-120: Inconsistent Prometheus registry usage.

This test creates a local p variable that shadows the package-level p defined in dualwriter_mode1_test.go. Other tests in this file (TestMode3_Create, TestMode3_List, TestMode3_Delete, TestMode3_DeleteCollection, TestMode3_Update) use the package-level p consistently.

♻️ Proposed fix to use package-level registry

 		if tt.setupStorageFn != nil {
 			tt.setupStorageFn(m, tt.input)
 		}

-		p := prometheus.NewRegistry()
 		dw := NewDualWriter(Mode3, ls, us, p)

---

37-44: Test case setup is misleading for Mode3 behavior.

The test case "error when creating object in the unified store fails" uses setupLegacyFn to configure the mock, but in Mode3, Storage is called synchronously first while Legacy is called asynchronously. Since both legacyStoreMock and storageMock share the same mock.Mock instance, this works by coincidence.

For clarity and to align with Mode3's semantics (Storage is primary), consider renaming the setup function or using setupStorageFn to make the test intent clearer.

♻️ Proposed fix to clarify test intent

 		{
 			name:  "error when creating object in the unified store fails",
 			input: failingObj,
-			setupLegacyFn: func(m *mock.Mock, input runtime.Object) {
+			setupStorageFn: func(m *mock.Mock) {
 				m.On("Create", mock.Anything, failingObj, mock.Anything, mock.Anything).Return(nil, errors.New("error"))
 			},
 			wantErr: true,
 		},

---

47-77: Consider verifying async legacy operations.

Mode3 delegates legacy writes to background goroutines. The current tests don't verify that the legacy store mock was called, which means the async behavior isn't being validated. If the legacy call fails silently or isn't executed at all, these tests wouldn't catch it.

Consider adding a short sleep or using mock.AssertExpectations with a timeout to verify the async legacy calls complete as expected.

pkg/apiserver/rest/dualwriter_mode3.go (1)

50-57: Async goroutines capture parent context which may be cancelled.

The background goroutines capture ctx from the parent scope and derive a timeout context from it via context.WithTimeoutCause. If the parent context gets cancelled after the function returns (e.g., when the HTTP request completes), the derived context will inherit that cancellation.

While this pattern appears intentional for fire-and-forget legacy writes and is consistent with Mode1, consider using context.WithoutCancel(ctx) (available in Go 1.21+) as the parent to ensure legacy operations aren't prematurely cancelled when the request completes.

Also applies to: 108-114, 134-141, 161-167